Skip to content

Let recipes use the model loaded in Chat#4840

Merged
danielhanchen merged 17 commits into
unslothai:mainfrom
wasimysaid:feat/local-model-provider
Apr 8, 2026
Merged

Let recipes use the model loaded in Chat#4840
danielhanchen merged 17 commits into
unslothai:mainfrom
wasimysaid:feat/local-model-provider

Conversation

@wasimysaid

@wasimysaid wasimysaid commented Apr 3, 2026

Copy link
Copy Markdown
Collaborator

What this does

Adds a "Local model" toggle to the Provider Connection block in the recipe editor. When you pick it, recipes automatically connect to whatever model you have loaded in the Chat tab. No endpoint, no API key, no setup.

The backend generates a short lived JWT and injects the local server endpoint right before the recipe subprocess starts. The data_designer library just sees a normal OpenAI compatible endpoint.

How it works

  1. User loads a model in Chat
  2. Opens recipe editor, adds a Provider Connection block
  3. Picks "Local model" instead of "External endpoint"
  4. Adds a Model Config block, links it to that provider (model ID auto fills)
  5. Runs the recipe and it just works

If no model is loaded the backend returns a clear error telling the user to load one first.

Changes

Backend (1 file)

  • jobs.py gets a _inject_local_providers helper that detects the is_local flag, checks a model is loaded, generates a 24h JWT with the existing admin subject, and fills in the loopback endpoint

Frontend (12 files)

  • is_local boolean added to ModelProviderConfig type, factory, payload builder, and import parser
  • Provider dialog gets a Local/External radio toggle. Picking local hides all the endpoint and key fields
  • Model config dialog auto fills model ID with "local" when linked to a local provider and clears it when switching back
  • Validation skips the endpoint check for local providers
  • Canvas inline node shows "Local model (Chat)" label instead of input fields

Known limitations

  • If the model gets unloaded or swapped while a recipe is running the requests will fail. The inference endpoint gives a clear error but the recipe doesnt retry. Fine for now
  • The JWT uses the admin user subject so if someone changes their password mid run the token invalidates. Unlikely in practice since recipes finish fast
  • Only one recipe can run at a time (existing constraint, not new)

Tested Manuelly

  • I want to manually test this before merging so opening as draft. Will go through the full flow with GGUF and transformers models, test the toggle states, import export round trip, and the error cases.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@unslothai unslothai deleted a comment from gemini-code-assist Bot Apr 3, 2026
@wasimysaid wasimysaid marked this pull request as ready for review April 7, 2026 09:45

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4c997e989a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/backend/routes/data_recipe/validate.py Outdated
Comment thread studio/backend/routes/data_recipe/jobs.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1b19db58e0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +106 to +108
parsed = urlparse(request_base_url)
port = parsed.port or 8888
endpoint = f"http://127.0.0.1:{port}/v1"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Derive local inference endpoint from backend bind port

This endpoint is computed from the client-facing request_base_url port (or 8888 fallback), which can differ from the backend's real listen port when Studio is behind a reverse proxy/TLS terminator. In that setup, local-provider jobs point to the wrong loopback URL (127.0.0.1:<external-port>/v1) and fail to connect even though Chat is loaded, so local mode becomes unreliable outside direct localhost access.

Useful? React with 👍 / 👎.

Comment on lines +90 to +94
onValueChange={(value) => {
const selectedProvider = value ?? "";
const isLocal = localProviderNames.has(selectedProvider);
if (isLocal && !config.model.trim()) {
onUpdate({ provider: selectedProvider, model: "local" });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply local-model reset when provider changes via blur

The new local/external model-id normalization only runs in onValueChange, but provider edits committed via free-text blur still take the old onBlur path and skip this logic. A common path is: select local provider (auto-fills model to local), then type an external provider name and blur—provider updates, but model stays local, which then gets sent to the external endpoint and causes avoidable run-time failures.

Useful? React with 👍 / 👎.

danielhanchen added 4 commits April 8, 2026 08:59
…provider fields, sync model id on toggle

Address review feedback on the local-model-provider flow:

- Backend (jobs.py): _resolve_local_v1_endpoint now reads the actual bound
  port from app.state.server_port (set in run.py after binding) instead of
  parsing it out of request.base_url, which is wrong behind any reverse
  proxy or non-default port. The two duplicated urlparse blocks are gone.
- Backend (jobs.py): defensively pop api_key_env, extra_headers, extra_body
  from local providers so a previously external provider that flipped to
  local cannot leak invalid JSON or rogue auth headers into the local /v1
  call. Also dedupe the post-loop assignment and tighten the local-name
  intersection so empty names cannot match.
- Backend (jobs.py): hoist datetime and urllib.parse imports to the top
  import block for consistency with the rest of the file.
- Backend (run.py): expose the bound port on app.state.server_port after
  the uvicorn server is constructed.
- Frontend (model-provider-dialog.tsx): clear extra_headers and extra_body
  when toggling to local mode. Hidden inputs would otherwise keep stale
  JSON blocking validate/run.
- Frontend (model-config-dialog.tsx): factor the local-aware provider
  selection logic into applyProviderChange and call it from both
  onValueChange and onBlur, so manually typing a provider name and tabbing
  away keeps the model field consistent.
- Frontend (recipe-studio.ts store): handle both directions of the
  is_local toggle in the cascade. external -> local now backfills
  model: "local" on already-linked model_configs so they pass validation
  immediately, mirroring the existing local -> external clear path.
- Frontend (validate.ts + build-payload.ts): thread localProviderNames
  into validateModelConfigProviders and skip the "model is required"
  check for local-linked configs. Local providers do not need a real
  model id since the inference endpoint uses the loaded Chat model.
…aph relink and node removal, harden ephemeral port path

Loop 2 review fixes:

- recipe-studio.ts: type-narrow next.is_local by also checking
  next.kind === "model_provider". TS otherwise raised TS2339 because
  next was typed as the union NodeConfig after the spread. The behavior
  is unchanged but the code now compiles cleanly.
- model-config-dialog.tsx: convert the lastProviderRef / providerInputRef
  ref-during-render pattern (pre-existing react-hooks/refs lint error)
  to a useEffect that syncs providerInputRef from config.provider. The
  combobox blur path still uses applyProviderChange and remains stable.
- recipe-graph-connection.ts: when a graph drag links a model_provider
  to a model_config, mirror the dialog applyProviderChange behavior:
  fill model: "local" if the new provider is local and the model field
  is blank, clear model when relinking from a local placeholder to an
  external provider, otherwise leave the model alone.
- reference-sync.ts: when a referenced provider node is removed, clear
  the synthetic model: "local" placeholder along with the provider
  field, so a future relink to an external provider does not pass
  validation with a stale value that fails at runtime.
- run.py: only publish app.state.server_port when the bound port is a
  real positive integer; for ephemeral binds (port==0) leave it unset
  and let request handlers fall back to request.base_url.
- jobs.py: _resolve_local_v1_endpoint also falls back when
  app.state.server_port is non-positive, and uses `is None` instead of
  the truthy fallback so a literal 0 is handled correctly.
…eachable configs, add scope-server port fallback

Loop 3 review fixes:

- jobs.py, validate.py: require `is_local is True` instead of truthy
  check. Malformed payloads such as is_local: "false" or is_local: 1
  would otherwise be treated as local and silently rewritten to the
  loopback endpoint.
- jobs.py: _resolve_local_v1_endpoint now tries request.scope["server"]
  (the actual uvicorn-assigned (host, port) tuple) as a second
  resolution step before falling back to parsing request.base_url.
  This covers direct-uvicorn startup paths and ephemeral binds that
  never publish app.state.server_port.
- jobs.py: new _used_llm_model_aliases helper collects the set of
  model_aliases that an LLM column actually references, and the
  "Chat model loaded" gate is now only triggered when a local
  provider is reachable from that set. Orphan model_config nodes on
  the canvas no longer block unrelated recipe runs.
…ON parsing for local providers, local-aware inline editor

Loop 4 review fixes:

- jobs.py: after rewriting local providers, also force
  skip_health_check: true on any model_config linked to a local
  provider. The /v1/models endpoint only advertises the real loaded
  model id, so data_designer's default model-availability health check
  would otherwise fail against the placeholder "local" id before the
  first chat completion call. The inference route already ignores the
  model id in chat completions, so skipping the check is safe.
- builders-model.ts: buildModelProvider now short-circuits for local
  providers and emits only { name, endpoint: "", provider_type, is_local }
  without running parseJsonObject on the hidden extra_headers/extra_body
  inputs. Imported or hydrated recipes with stale invalid JSON in those
  fields no longer block client-side validate/run.
- inline-model.tsx: the model_config branch now accepts an optional
  localProviderNames prop and mirrors the dialog applyProviderChange
  behavior. Changing provider to/from a local one auto-fills or clears
  the "local" placeholder consistently with the other edit paths.
- recipe-graph-node.tsx: derive localProviderNames from the store via
  useMemo (stable identity) and pass it through renderNodeBody to
  <InlineModel>. Hooks order is preserved by declaring them above the
  early return for markdown_note nodes.
- run.py: minor comment tweak - loop 3 already added the scope-server
  fallback path, note that in the comment.
@danielhanchen danielhanchen merged commit 8e97744 into unslothai:main Apr 8, 2026
1 check passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d21b07714c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +81 to +82
if provider.pop("is_local", None) is True:
provider["endpoint"] = "http://127.0.0.1"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Align local-provider validation patch with job injection

The validation path only rewrites local providers to endpoint="http://127.0.0.1", but it does not apply the same local-mode mutations used in create_job (JWT injection and skip_health_check on linked model configs). Because validate() still calls validate_recipe(recipe), local recipes that use placeholder model IDs like "local" can fail preflight model/provider checks during “Check recipe” even though the run path succeeds after _inject_local_providers mutates the payload.

Useful? React with 👍 / 👎.

const config = useRecipeStudioStore((state) => state.configs[id]);
const openConfig = useRecipeStudioStore((state) => state.openConfig);
const updateConfig = useRecipeStudioStore((state) => state.updateConfig);
const allConfigs = useRecipeStudioStore((state) => state.configs);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop subscribing each graph node to global configs

This selector makes every RecipeGraphNodeBase subscribe to the entire configs object, so any config update (including editing a single node) invalidates all node subscriptions and rerenders the whole canvas. On larger recipes this introduces avoidable O(N) rerender churn and noticeably degrades editor responsiveness; derive local-provider names once at a higher level or use a narrower selector.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants